-
Notifications
You must be signed in to change notification settings - Fork 193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better Search Results #1871
base: main
Are you sure you want to change the base?
Better Search Results #1871
Conversation
f51334a
to
291b5a3
Compare
@Lagoja Can someone look at this? I believe that this is a much better search experience for users. Any feedback would be appreciated. |
@Lagoja Also, I don't recall the package at the moment, but the current code (and this PR) filter out packages with empty versions. I think that might be wrong, as I think empty version implies "latest" and by filtering them out they are unfindable. |
We appreciate the PR, but I think this change may require a bit more discussion because it has a large impact on the UI. We also import a couple TUI-like helper libraries already (some indirectly like charmbracelet), so it would be nice to consolidate instead of adding another dependency. |
@gcurtis It would be great if some of that discussion could occur here. I'm game to make changes to get a tweaked version of this PR in, as I believe it greatly enhances the usability to the search command. Also, maybe I missed it, but looks like only lipgloss is currently used from charmbracelet. |
Yeah, we actually don't pull in any core charmbracelet libraries because they are very large. They would probably increase binary released size by 10MB+ |
That makes sense. For reference, adding the dependency used here only increased the binary size for Darwin arm64 by 164kb. |
Looking at this, my thoughts are:
As an example, if I run Is there a way to achieve the version grouping while maintaining the compactness of the current output? |
@Lagoja Thanks for the feedback. The width is intentionally constrained at the moment. The platforms list could be two columns to reduce the height somewhat. The table dividers, or something like that, seem necessary to be able to unambiguously identify which versions and platforms apply together. Color would be an option, but then it might not work in different terminals or with color blindness. Another improvement would be to detect the width of the terminal and lay the table out at the width. That would allow more columns for version and platform. Also, once a package stops flip-flopping between supported platforms the version will just appear in the head row, so that saves a ton of space. |
Would it make sense to have the output paginate by default? That way the most relevant hit is shown first, instead of having to scroll up. |
It could. Normal Unix would be to either pipe to a pager or use $PAGER to do the paging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, my proposal is:
- Let's implement pagination. I think it's important to have the most relevant result in the search show up first.
- @mikeland73 let's put this behind a feature flag, and advertise it in the upcoming pre-release. If we test it and like it, we can promote it to a default feature.
Thoughts?
e797404
to
4e73e8d
Compare
I'm game to take a stab at implementing paging if you are all in agreement that it will be a useful addition. |
@Lagoja I was considering relying on the system pager ($PAGER) or an devbox specific config ($DEVBOX_PAGER). The could would look like:
|
This no longer works because v1/search endpoint stopped returning all platforms. To get it to work you can do:
|
Summary
Output tabular search results which show the platforms available
How was it tested?
Ran search locally and looked at the output.